Add an optional class filter to JavaSerializer deserialization - #1295

Hi @Nexory. Thanks for the PR. The issue I see is that your filter is applied to late. It does Class.forName and then* checks if the class is allowed. The potentially malicious class is already loaded at this point. The check would need to be done before class loading and match on the class name.

Kryo 6 will move to Java 17 or 21 and we could replace this with an ObjectInputFilter then.

You are right that the check belongs before the class is resolved, and it now is: the filter
runs on type.getName() as the first thing resolveClass does, so a refused class is never
loaded and a name that does not resolve at all is refused rather than reported as missing. The
type changes from Predicate<Class> to Predicate<String> accordingly.

One correction to the reasoning, because it affects how urgent this was. The old code called
Class.forName(name, false, loader), and that false means do not initialize. Measured with a
class whose static initializer prints: resolving it that way does not run the initializer,
while Class.forName(name) does. So the class was being loaded but no attacker code ran at
that point. The change is still the right one, for the reasons above.

Measured with -verbose:class, on a gadget nested inside an allowed outer type, since that is
where the filter applies: with the filter refusing, the class does not appear in the load list
and its initializer does not run; with the filter allowing, both happen. The reader never names
the class and the bytes come from a separate JVM.

Worth noting for the Kryo 6 plan: this is the same shape ObjectInputFilter uses, a decision
on the name before resolution, so swapping the implementation later should be small.

I followed your PR workflow: build on JDK 11, then mvn -B test on 8 and on 17. 310 and 330
tests, no failures. I did not run the 11, 21, 25 and 26 legs.

One correction to the reasoning, because it affects how urgent this was. The old code called
Class.forName(name, false, loader), and that false means do not initialize. Measured with a
class whose static initializer prints: resolving it that way does not run the initializer,
while Class.forName(name) does. So the class was being loaded but no attacker code ran at
that point. The change is still the right one, for the reasons above.

You are right! I overlooked the false parameter. Now I'm not so sure anymore what the ideal shape of the filter should be. ObjectInputFilter can be constructed from a String pattern as well as a Predicate<Class>, but the final check is always against a FilterInfo which exposes a Class. So arguably, a Predicate<Class> is closer to ObjectInputFilter.

It's unfortunate that we are still on JDK8 and can't go for ObjectInputFilter directly.

What's your opinion? Should we revert back to your original version, or should we go with String?